Repository navigation
feat(otel-collector): add statsdreceiver for StatsD/DogStatsD ingestion - #3100
Conversation
Compiles in `statsdreceiver` so StatsD and DogStatsD-formatted metrics can be ingested directly, without a separate StatsD-to-OTLP bridge. Documents that it doesn't support horizontally-scaled deployments (a real, upstream-documented limitation, not HyperDX-specific) and that it should not be wired into the OpAMP-managed otel-collector Deployment, which runs multiple replicas for OTLP ingest availability. Verified against the real compiled binary (pinned v0.155.0): confirmed `statsdreceiver` is not currently compiled in, added it, rebuilt, and confirmed the documented config both passes `otelcontribcol validate --config` and actually starts (`otelcontribcol --config ...`, reaching "Everything is ready. Begin running and processing data."), using the same multi-config merge the container's own entrypoint uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@arj22 is attempting to deploy a commit to the HyperDX Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 2712e01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Deep ReviewScope: PR #3100 — Verified directly against the checkout: no default config ( ✅ No critical issues found. This is a low-risk, additive, opt-in change; the compose/Dockerfile/README changes are internally consistent and prior review feedback on port publication, DogStatsD 🟡 P2 — recommended
🔵 P3 nitpicks (1)
Reviewers (6): correctness, testing, maintainability, project-standards, reliability, previous-comments. Testing gaps:
No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PR Review
3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor 3 posted as inline comment(s) on the changed lines. Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
…laim - Removed the timer_histogram_mapping example claiming to route "timer/histogram-type" metrics to an exponential histogram. Verified against the pinned v0.155.0 source (factory.go's defaultTimerHistogramMapping): timer, histogram, AND distribution all default to a gauge observer, and the example only remapped `timing` - histogram/distribution metrics would still land as gauges, contradicting the comment. - Reframed the section around plain StatsD rather than DogStatsD specifically - the receiver handles any StatsD client; DogStatsD's tag extension is supported but not required, and centering the docs on one vendor's dialect was misleading and pulled in the now-removed, Datadog-distribution-specific histogram-mapping example. - Net simplification: fewer lines, no vendor-specific framing, no incorrect claim. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e_simple_tags - Adding statsd to the default `metrics` pipeline via CUSTOM_OTELCOL_CONFIG_FILE silently no-ops in OpAMP supervisor mode (HyperDX's default deployment): opampController.ts overwrites that pipeline's receivers/exporters at runtime the same way it does for spanmetricsconnector (see the span metrics section above, ClickHouse#2351). Added a second example using a dedicated `metrics/statsd` pipeline name, which the remote config never touches, mirroring the existing OpAMP-mode span_metrics example. Verified against the real compiled binary by reconstructing opampController's generated base config as a static fixture and running `validate --config` against the merge. - Corrected the multi-replica warning: it was written as if OpAMP mode itself implies multiple replicas, but HyperDX's shipped default deployments run the OpAMP-managed collector as a single instance — the actual constraint is scaling *whichever* collector ingests statsd beyond one replica, independent of which mode it runs in. - Restored `enable_simple_tags: true` to both examples and qualified the intro claim about DogStatsD tag support: valued tags work with no extra config, but bare tags (`#mytag`) are silently dropped without this option. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
StatsD defaults to UDP, and neither the shipped compose files nor the collector/all-in-one Dockerfiles published or exposed 8125 - a user who added the documented statsd config verbatim on top of the default stack got no data and no error (UDP sends are fire-and-forget, so nothing logs the drop). Publish `8125:8125/udp` on the otel-collector service in docker-compose.yml and docker-compose.dev.yml (parameterized via HDX_DEV_OTEL_STATSD_PORT, matching the other dev port overrides), and add 8125/udp to the EXPOSE lines in docker/otel-collector/Dockerfile (both dev and prod stages) and docker/hyperdx/Dockerfile (all-in-one). statsd itself stays opt-in (nothing listens on the port until a pipeline config adds the receiver), so this is a no-op for anyone not using it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An unconditional fixed host port mapping fails the whole otel-collector service's startup if anything else already owns 8125/udp (a local StatsD daemon or Datadog Agent commonly does) - since statsd itself is opt-in and off by default, this would have broken the default stack for anyone not using the feature, just for having it in their environment. Commented the mapping out by default in both compose files, matching the existing opt-in-port convention already used in the same files (db's 27017, ch-server's 8123/9000). Uncommenting it remains a one-line change, documented in the README section.
|
@wrn14897 this is ready for review. |
| # packages/otel-collector/README.md). Left commented out by default: | ||
| # a fixed host port mapping fails the whole service's startup if | ||
| # something else on the host already owns 8125/udp. | ||
| # - '${HDX_DEV_OTEL_STATSD_PORT:-8125}:8125/udp' # statsd receiver |
There was a problem hiding this comment.
🔵 minor — HDX_DEV_OTEL_STATSD_PORT is never allocated per dev slot, so it hardcodes 8125 for every worktree
Every other HDX_DEV_*_PORT in this file is assigned a per-slot value in scripts/dev-env.sh (HDX_DEV_OTEL_JSON_HTTP_PORT=$((31100 + HDX_DEV_SLOT)), line 57) and documented in the port table in agent_docs/development.md:63-67. HDX_DEV_OTEL_STATSD_PORT appears in neither, so the :-8125 fallback always wins and two worktrees that both uncomment this line collide on the host — exactly the startup failure the comment says it is avoiding. Add HDX_DEV_OTEL_STATSD_PORT=$((31200 + HDX_DEV_SLOT)) plus its export in scripts/dev-env.sh and a row in the agent_docs/development.md table.
| - '4318:4318' # OTLP http receiver | ||
| - '8888:8888' # metrics extension | ||
| # Uncomment to enable statsd ingestion (also requires adding the | ||
| # `statsd` receiver to a pipeline via CUSTOM_OTELCOL_CONFIG_FILE - see |
There was a problem hiding this comment.
🔵 minor — docker-compose.yml's otel-collector has no CUSTOM_OTELCOL_CONFIG_FILE env var or config mount, so the instruction can't be followed
Unlike docker-compose.dev.yml (which sets CUSTOM_OTELCOL_CONFIG_FILE at line 39 and bind-mounts custom.config.yaml at line 52), this service declares no volumes: and no such env var, so a user who uncomments the port has nowhere to put the statsd receiver block. Add the matching commented-out environment:/volumes: lines here (or in the README's statsd section) so the opt-in is actually completable in the shipped production compose.
| (not merges) those keys — see the [span metrics section | ||
| above](#computing-span-metrics-from-ingested-traces) for the same | ||
| behavior. Adding `statsd` to `metrics.receivers` from | ||
| `CUSTOM_OTELCOL_CONFIG_FILE` gets silently dropped: no error, the receiver |
There was a problem hiding this comment.
🔵 minor — README says an unwired statsd receiver "just starts"; an unreferenced receiver is never instantiated
When the OpAMP remote config overwrites metrics.receivers, the statsd receiver is left referenced by no pipeline, and the collector never builds or starts it — nothing binds 8125/udp at all. The equivalent span-metrics paragraph gets this right ("the connector is just compiled in and wired to nothing", line 282). Reword to match, since the current text sends a debugging user toward the port mapping instead of the pipeline.
| statsd: | ||
| # Defaults to localhost:8125, which only accepts local traffic - | ||
| # 0.0.0.0 is required here since traffic arrives from outside the | ||
| # container (the published port above forwards to the container's |
There was a problem hiding this comment.
🔵 minor — The parenthetical explaining why 0.0.0.0 is needed states the mechanism backwards
A published port is DNAT'd to the container's bridge interface, not to the container's loopback — which is precisely why a receiver bound to localhost:8125 is unreachable. As written ("the published port above forwards to the container's own loopback otherwise") it implies the default localhost binding would work, contradicting the advice it supports. Reword to "traffic arrives on the container's external interface, not its loopback".
| OpAMP-managed collector as a single instance, so this doesn't bite out of | ||
| the box — it only matters once *some* collector instance ingesting | ||
| `statsd` is scaled to multiple replicas, at which point that traffic | ||
| needs to go to one dedicated single-replica instance instead. |
There was a problem hiding this comment.
🔵 minor — New section doesn't note that statsd has no authentication, unlike every other exposed receiver
packages/api/src/opamp/controllers/opampController.ts:361-371 and :397-406 attach bearertokenauth to otlp/hyperdx and datadog when collector auth is enforced, and the Datadog section here has a dedicated "Authentication" subsection. statsdreceiver supports no authenticator, so the recommended endpoint: 0.0.0.0:8125 plus a published 8125:8125/udp is an unauthenticated, spoofable metrics ingest path even in an auth-enforced deployment. Add a sentence saying so and recommending the port be restricted at the network layer (e.g. bind the host side to an internal address).
|
@arj22 sorry for the delay, it looks like the smoke tests are failing though. Could you take a look at that please? |
|
@pulpdrew Looked into it — likely not caused by this PR, the diff here doesn't touch ClickHouse or the smoke-test setup. The failure was |
## Summary Fixes the otel smoke tests, which fail because #3142 added a prometheus related ClickHouse config property that causes the (deliberately older) ClickHouse compat version used in the otel smoke tests to fail on startup. This is fixed by removing the prometheus setting from the compat container's config. To prevent this in the future, the conditions for running the smoke tests have been widened to include changes to the ClickHouse config. Related: Smoke tests are failing on #3100. ### Screenshots or video No behavior changes. ### How to test on Vercel preview No behavior changes. ### References - Linear Issue: Closes HDX-5434 - Related PRs:
Summary
Adds the OpenTelemetry
statsdreceiverto HyperDX's custom collector build, so users can ingest StatsD metrics directly without a separate bridge. Purely additive/opt-in — no default pipeline or behavior changes.The docs call out one real limitation from upstream: this receiver keeps aggregation state per-instance with no cross-replica coordination, so it shouldn't be wired into HyperDX's multi-replica
otel-collectorDeployment — it needs a dedicated single-replica instance.Test plan
statsdreceiverwasn't previously compiled in, then confirmed it's present after the change (otelcontribcol components).validate.🤖 Generated with Claude Code